Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: build windows and linux binaries with fips enabled Microsoft Go Fork #4770

Merged
merged 1 commit into from
Aug 11, 2023
Merged

feat: build windows and linux binaries with fips enabled Microsoft Go Fork #4770

merged 1 commit into from
Aug 11, 2023
+400 −175

Conversation

asaf92
Copy link

@asaf92 asaf92 commented Aug 2, 2023 •
edited by PeterSchafer
Loading

What does this PR do?

  • This PR introduces a new way to download and install the golang version used for building the CLI.
  • This enables to use different golang versions besides the google version, we are looking towards using the MS Go Fork in the future.
  • It also includes some pipeline refactorings to unify the build job.
  • In addition it supports detecting the GOEXPERIMENT values introduced in the MS Go Fork to enable FIPS validated crypto.

Where should the reviewer start?

The different build steps should include log messages like this

-- Pre-paring to build
--  Using Go:        ***/go/bin/go
--  Building Binary: ***/snyk/cliv2/_bin/snyk-linux
--  GOEXPERIMENT:

The download go binary step should include
--base_url=https://go.dev/dl/

How should this be manually tested?

Currently this doesn't include a behavioural or functional change.

Any background context you want to provide?

What are the relevant tickets?

HEAD-392, HEAD-395

Screenshots

PeterSchafer
PeterSchafer reviewed Aug 2, 2023
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
@Avishagp Avishagp force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch 13 times, most recently from 9614ee9 to 2c5dc87 Compare August 4, 2023 15:28
@asaf92 asaf92 marked this pull request as ready for review August 6, 2023 07:50
@asaf92 asaf92 requested a review from a team as a code owner August 6, 2023 07:50
@asaf92 asaf92 force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from dc7ade4 to 1892ff6 Compare August 6, 2023 08:24
@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch 2 times, most recently from f3bf7fa to c2b95db Compare August 7, 2023 11:45
@j-luong j-luong requested a review from a team as a code owner August 7, 2023 16:36
@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from 0a935e2 to f43160d Compare August 7, 2023 18:05
@j-luong j-luong mentioned this pull request Aug 8, 2023
Closed
1 task
@PeterSchafer PeterSchafer changed the title chore: change Go build to the microsoft fork feat: build windows and linux binaries with fips enabled Microsoft Go Fork Aug 8, 2023
@j-luong j-luong force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from 74c60e4 to a346f4e Compare August 8, 2023 15:04
@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch 4 times, most recently from cfd0e5d to 71485ed Compare August 9, 2023 09:22
j-luong
j-luong reviewed Aug 9, 2023
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
j-luong
j-luong reviewed Aug 9, 2023
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
bastiandoetsch
bastiandoetsch reviewed Aug 10, 2023
View reviewed changes
cliv2/Makefile Outdated Show resolved Hide resolved
bastiandoetsch
bastiandoetsch reviewed Aug 10, 2023
View reviewed changes
cliv2/Makefile Outdated Show resolved Hide resolved
bastiandoetsch
bastiandoetsch reviewed Aug 10, 2023
View reviewed changes
scripts/download_go.py Outdated
print("Downloading " + url + " to " + tmp_file)

try:
urllib.request.urlretrieve(url, tmp_file)
Copy link
Contributor

@bastiandoetsch bastiandoetsch Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using gzip encoding: https://gist.github.com/Manouchehri/0ce55d239fb07c41c92f. Alternatively, the requests library does it automatically. On the other hands, I just saw that it's compressed files anyway, so the benefits would be minuscule.

bastiandoetsch
bastiandoetsch reviewed Aug 10, 2023
View reviewed changes
scripts/download_go.py

try:
urllib.request.urlretrieve(url, tmp_file)
print("Download complete")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shasum checking would be awesome!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, as we're talking about FIPS functionality, do the go providers provide GPG keys to check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's possible from big G. we can fetch the asc like so:
https://storage.googleapis.com/golang/go1.20.linux-amd64.tar.gz.asc

It's not well documented, but the public key to verify looks like it can be fetched like so:
https://dl.google.com/dl/linux/linux_signing_key.pub

MSFT has signatures available via their download URL, e.g. https://aka.ms/golang/release/latest/go1.20.linux-amd64.tar.gz.sig

And can be validated using their public key:

Validate GPG signatures using our public key:

https://download.microsoft.com/download/f/a/2/fa2420dd-3f08-4621-82cf-5a22abcbc8f9/microsoft-managed-lang-compiler.asc
Shortened link for sharing: https://aka.ms/managed-lang-compiler-public-key

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets do gpg in a follow-up PR/Story, using aharc-coded public keys in config.yaml and validate on each platform using the given key.

@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from 08e776c to 052667a Compare August 10, 2023 18:18
@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from 052667a to 1333871 Compare August 11, 2023 07:30
@j-luong @Avishagp @PeterSchafer
feat: use MS Go fork for windows and linux builds
4ced277 
Co-authored-by: JSON <jason.luong@snyk.io>
Co-authored-by: Avishag Israeli <44115709+Avishagp@users.noreply.github.com>
@PeterSchafer PeterSchafer force-pushed the feat/use-microsoft-go-fork-on-windows-head-395 branch from 1333871 to 4ced277 Compare August 11, 2023 12:50
@PeterSchafer PeterSchafer merged commit 5f7c202 into master Aug 11, 2023
@PeterSchafer PeterSchafer deleted the feat/use-microsoft-go-fork-on-windows-head-395 branch August 11, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Avishagp Avishagp Avishagp left review comments

@j-luong j-luong j-luong approved these changes

@PeterSchafer PeterSchafer PeterSchafer approved these changes

@bastiandoetsch bastiandoetsch bastiandoetsch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@asaf92 @j-luong @bastiandoetsch @Avishagp @PeterSchafer